Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate leo and tpi executables #662

Merged
merged 20 commits into from
Oct 10, 2022
Merged

Separate leo and tpi executables #662

merged 20 commits into from
Oct 10, 2022

Conversation

tasdomas
Copy link
Contributor

@tasdomas tasdomas commented Sep 9, 2022

No description provided.

Copy link
Member

@0x2b3bfa0 0x2b3bfa0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To my mind, moving the Terraform Provider entry point under ./cmd/tpi makes no sense from a project structure standpoint. Note that this repository is first and foremost a Terraform Provider.

I'd wait to refine leo user experience a bit more and then suggest migrating it to a different module, so it can be easily installed with go install like any other tool, and the iterative_task Terraform Resource can still depend on the underlying packages.

@tasdomas
Copy link
Contributor Author

To my mind, moving the Terraform Provider entry point under ./cmd/tpi makes no sense from a project structure standpoint. Note that this repository is first and foremost a Terraform Provider.

I'd wait to refine leo user experience a bit more and then suggest migrating it to a different module, so it can be easily installed with go install like any other tool, and the iterative_task Terraform Resource can still depend on the underlying packages.

Ok, I've moved tpi back into project root.

@tasdomas tasdomas temporarily deployed to automatic September 19, 2022 09:35 Inactive
@tasdomas tasdomas requested a deployment to automatic September 19, 2022 09:35 Abandoned
@tasdomas tasdomas requested a deployment to automatic September 19, 2022 09:35 Abandoned
@tasdomas tasdomas requested a deployment to automatic September 19, 2022 09:35 Abandoned
@tasdomas tasdomas temporarily deployed to automatic September 19, 2022 09:35 Inactive
@tasdomas tasdomas changed the title Separate leo and tpi executables. Separate leo and tpi executables Oct 4, 2022
@0x2b3bfa0
Copy link
Member

As stated at #662 (review), prematurely separating binaries in the same repository might not add too much value, and it's probably more important to separate the repositories themselves. 🤔

@0x2b3bfa0 0x2b3bfa0 temporarily deployed to automatic October 4, 2022 17:53 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to automatic October 4, 2022 17:53 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to automatic October 4, 2022 17:53 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to automatic October 4, 2022 17:53 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to automatic October 4, 2022 17:53 Inactive
@tasdomas
Copy link
Contributor Author

tasdomas commented Oct 5, 2022

I'd argue that the binaries were combined prematurely. And that is causing a bug.

@0x2b3bfa0
Copy link
Member

0x2b3bfa0 commented Oct 9, 2022

I'd argue1 that whether binaries are combined or not is not the main issue, but trying to shoehorn Terraform HCL on a Cobra & Viper command-line tool was definitely premature (or, rather, something that shouldn't have happened)

Footnotes

  1. Although agreeing is a strange way of arguing

@0x2b3bfa0 0x2b3bfa0 temporarily deployed to automatic October 10, 2022 03:37 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to automatic October 10, 2022 03:38 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to automatic October 10, 2022 03:38 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to automatic October 10, 2022 03:38 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to automatic October 10, 2022 03:38 Inactive
@0x2b3bfa0
Copy link
Member

I don't want to stop this pull request for the sake of perfectionism; it's indeed a step in the right direction, degree more or less 🌏 🌍 🌎

@0x2b3bfa0 0x2b3bfa0 enabled auto-merge (squash) October 10, 2022 03:39
@0x2b3bfa0 0x2b3bfa0 merged commit a1a5092 into master Oct 10, 2022
@0x2b3bfa0 0x2b3bfa0 deleted the d015-tpi-leo-separate branch October 10, 2022 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants